Skip to content

Conversation

AnnaShepa
Copy link
Collaborator

Added multiple_selection field test to test_very_parametrized_script.py
Added search and history tests

Added multiple_selection field test to test_very_parametrized_script.py
@bugy
Copy link
Owner

bugy commented Jun 2, 2021

Copy link
Owner

@bugy bugy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove UI only tests, please? They are easier and faster to test with normal UI tests
For e2e tests I would expect communication with a server (or that UI is built properly based on server responses)


@property
def all_groups(self):

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the empty line


@property
def history_table_column_titles(self):
return ["ID", "Start Time", "User", "Script", "Status"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be a constant?


@property
def history_table(self):
return self.find_element("div.executions-log-table tr")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you return "tr" as a "history table" ? tr is only a single row
Please either remove the tr from here or rename the method name

home_page.load()

home_page.sidebar_search_button.click()
home_page.sidebar_search_input.send_keys(search_request)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace the variable with the string literal, it makes the test easier to read

displayed_results = home_page.all_script_links + home_page.all_groups

for result in displayed_results:
expect(str(result.text).find(search_request) > -1, "\"{}\" containts no search request \"{}\" but still listed after search".format(str(result.text), search_request))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are working with a predefined set of scripts, I think it would be more transparent to verify, that "display_result" is equal to an array of some elements

very_parametrized_script_page = VeryParametrizedScript(browser, config_host)
very_parametrized_script_page.parameter_multiple_selection.click()

expect(is_displayed(very_parametrized_script_page.parameter_multiple_selection_drop_down), "Drop down for Multiple selection not found after click")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this test? It would be covered by the next test anyway

expect("selected" not in random_option.get_attribute("class").split(" "), "Checked options stays selected after click on it")
random_option.click()
expect("selected" in random_option.get_attribute("class").split(" "), "Unchecked options stays not selected after click on it")
if len(very_parametrized_script_page.parameter_multiple_selection_drop_down_unchecked_elements) > 0:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a blank line before this if

def test_check_multiple_selection_check_uncheck_action(browser, config_host):
very_parametrized_script_page = VeryParametrizedScript(browser, config_host)

if len(very_parametrized_script_page.parameter_multiple_selection_drop_down_checked_elements) > 0:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are working with predefined scripts, I think this if is redundant here. You should know what to expect there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants